Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set up NSView the way Glutin requires it #1698

Merged
merged 3 commits into from
Sep 11, 2024
Merged

Conversation

madsmtm
Copy link
Member

@madsmtm madsmtm commented Sep 11, 2024

In particular Glutin requires wantsBestResolutionOpenGLSurface and wantsLayer to be set correctly.

This is currently done by Winit:

https://github.com/rust-windowing/winit/blob/dfea49f48850670cdfe3dc91949a9f8f2e267a38/src/platform_impl/apple/appkit/window_delegate.rs#L640-L653

But it's really the job of Glutin to do this, Winit shouldn't be concerned with OpenGL-specific details. I'm going to submit a PR later to Winit to remove the code from there.


I decided to avoid the option to choose whether to do High DPI or not, like the with_disallow_hidpi flag in Winit, since I don't see why we should let the user control this? It was originally added in rust-windowing/winit#821

I only have a device running macOS 10.14, so I can't actually test the behaviours here. But we're just mirroring what Winit is doing.

In particular `wantsBestResolutionOpenGLSurface` and `wantsLayer`.

This is currently done by Winit, but it's really the job of Glutin to do
this, Winit shouldn't be concerned with OpenGL-specific details.
@kchibisov
Copy link
Member

Yeah, it should be done by glutin indeed.

Btw, I've seen your PR to wgpu about adding a layer, do you happen to know if glutin needs something like that as well? Since it also generally lags on resize, etc.

Also, I'd assume that it doesn't need to be synced with winit change, because if glutin will start doing it, it won't really change anything if winit e.g v0.30 does it and v0.31 not?

@madsmtm
Copy link
Member Author

madsmtm commented Sep 11, 2024

Btw, I've seen your PR to wgpu about adding a layer, do you happen to know if glutin needs something like that as well? Since it also generally lags on resize, etc.

I can't remember the result of my investigation right now, it's been a few months, but I'm looking at doing a PR for (finally) changing how redraws work in Winit on macOS/iOS, and I'll include a summary of all the crates I've tested with, if something is needed in Glutin then I'll open a PR here as well before that.

Also, I'd assume that it doesn't need to be synced with winit change, because if glutin will start doing it, it won't really change anything if winit e.g v0.30 does it and v0.31 not?

Indeed, we can do this now, and change it in Winit later.

Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a changelog for that?

I'd also assume that you've tested it with winit at least or OpenGL doesn't generally work for you?

I'm thinking to backport this to older glutin versions as well, so it won't break with new winit, etc, since glutin doesn't require that, generally.

@madsmtm
Copy link
Member Author

madsmtm commented Sep 11, 2024

Could you add a changelog for that?

I didn't originally, because the template says:

Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users

And this isn't really gonna be valuable to users, it's mostly an internal change between Glutin and Winit. But I'll try.

I'd also assume that you've tested it with winit at least or OpenGL doesn't generally work for you?

It works fine on my Mac, but again, I can't really test the behaviour on the older versions, as I don't have a device that runs on old enough macOS versions.

I'm thinking to backport this to older glutin versions as well, so it won't break with new winit, etc, since glutin doesn't require that, generally.

That's up to you.

@kchibisov
Copy link
Member

And this isn't really gonna be valuable to users, it's mostly an internal change between Glutin and Winit. But I'll try.

using glutin doesn't imply that you're using winit, so, it's valuable for the users since it may alter things for consumers, glutin assuming winit behavior is a bug from my point of view, since it generally should setup things the way it wants and not just rely that external nsview provider will do that for it.

@kchibisov kchibisov merged commit 1b30cd7 into master Sep 11, 2024
43 checks passed
@kchibisov kchibisov deleted the madsmtm/macos-nsview-setup branch September 11, 2024 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants